-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[libc] Disabled mpfr tests in full build mode, even if we set the mpfr path #149655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…r path If we are doing a full libc build, mpfr tests should be disabled even if we set the mpfr path. This should fix the rv32 buildbot.
@llvm/pr-subscribers-libc Author: Mikhail R. Gadelha (mikhailramalho) ChangesIf we are doing a full libc build, mpfr tests should be disabled even if we set the mpfr path. This patch simply swaps the check order. This should fix the rv32 buildbot. Full diff: https://github.com/llvm/llvm-project/pull/149655.diff 1 Files Affected:
diff --git a/libc/cmake/modules/LLVMLibCCheckMPFR.cmake b/libc/cmake/modules/LLVMLibCCheckMPFR.cmake
index 95815deb07298..c32e25e9d5f50 100644
--- a/libc/cmake/modules/LLVMLibCCheckMPFR.cmake
+++ b/libc/cmake/modules/LLVMLibCCheckMPFR.cmake
@@ -1,11 +1,11 @@
set(LLVM_LIBC_MPFR_INSTALL_PATH "" CACHE PATH "Path to where MPFR is installed (e.g. C:/src/install or ~/src/install)")
-if(LLVM_LIBC_MPFR_INSTALL_PATH)
- set(LIBC_TESTS_CAN_USE_MPFR TRUE)
-elseif(LIBC_TARGET_OS_IS_GPU OR LLVM_LIBC_FULL_BUILD)
+if(LIBC_TARGET_OS_IS_GPU OR LLVM_LIBC_FULL_BUILD)
# In full build mode, the MPFR library should be built using our own facilities,
# which is currently not possible.
set(LIBC_TESTS_CAN_USE_MPFR FALSE)
+elseif(LLVM_LIBC_MPFR_INSTALL_PATH)
+ set(LIBC_TESTS_CAN_USE_MPFR TRUE)
else()
try_compile(
LIBC_TESTS_CAN_USE_MPFR
|
if(LLVM_LIBC_MPFR_INSTALL_PATH) | ||
set(LIBC_TESTS_CAN_USE_MPFR TRUE) | ||
elseif(LIBC_TARGET_OS_IS_GPU OR LLVM_LIBC_FULL_BUILD) | ||
if(LIBC_TARGET_OS_IS_GPU OR LLVM_LIBC_FULL_BUILD) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will also disable MPFR tests in full build for other architectures?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, If you set -DLLVM_LIBC_FULL_BUILD=ON
, it will already disable mpfr tests, unless you explicitly set LLVM_LIBC_MPFR_INSTALL_PATH
.
I double-checked all buildbots and the only one that was doing a full build and setting the mpfr path was the rv32 buildbot: https://github.com/llvm/llvm-zorg/blob/6fcd3566a6c9665f70f0bf3f366a148c38787a25/zorg/buildbot/builders/annotated/libc-linux.py#L112
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we update the RV32 bot command instead? It was intentional to so that mpfr tests can be run in full build mode when needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can, but the way the code is today you won't be able to build in full build mode and set the mpfr path at the same time in any arch, or it will fail with the same error we are seeing on rv32 now.
This is my log from trying to build on x86, with the following cmake command from the llvm libc website:
$ cmake ../runtimes \
-G Ninja \
-DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++ \
-DLLVM_ENABLE_RUNTIMES="libc;compiler-rt" \
-DLLVM_LIBC_FULL_BUILD=ON \
-DCMAKE_BUILD_TYPE=Debug \
-DLLVM_LIBC_INCLUDE_SCUDO=OFF \
-DCOMPILER_RT_BUILD_SCUDO_STANDALONE_WITH_LLVM_LIBC=OFF \
-DCOMPILER_RT_BUILD_GWP_ASAN=OFF \
-DCOMPILER_RT_SCUDO_STANDALONE_BUILD_SHARED=OFF \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
-DLLVM_ENABLE_SPHINX=OFF -DLIBC_INCLUDE_DOCS=OFF \
-DLIBC_CMAKE_VERBOSE_LOGGING=ON \
-DLLVM_LIBC_MPFR_INSTALL_PATH=/usr/` # set mpfr path
$ ninja libc.test.src.__support.FPUtil.bfloat16_test.__unit__
[66/75] Building CXX object libc/test/src/__support/FPUtil/CMakeFiles/libc.test.src.__support.FPUtil.bfloat16_test.__unit__.__build__.dir/bfloat16_test.cpp.o
FAILED: libc/test/src/__support/FPUtil/CMakeFiles/libc.test.src.__support.FPUtil.bfloat16_test.__unit__.__build__.dir/bfloat16_test.cpp.o
/home/mgadelha/tools/llvm-project/build/bin/clang++ -DLIBC_NAMESPACE=__llvm_libc_22_0_0_git -D_DEBUG -I/home/mgadelha/tools/llvm-project/libc -isystem /home/mgadelha/tools/llvm-project/buildlibc/libc/include -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -g -std=gnu++17 -mavx2 -mfma -DLIBC_QSORT_IMPL=LIBC_QSORT_QUICK_SORT -DLIBC_ADD_NULL_CHECKS -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_DEFAULT -fpie -DLIBC_FULL_BUILD -ffreestanding -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -ffixed-point -Wconversion -Wno-sign-conversion -Wimplicit-fallthrough -Wwrite-strings -Wno-c99-extensions -Wno-gnu-imaginary-constant -Wno-pedantic -Wstrict-prototypes -Wextra-semi -Wnewline-eof -Wnonportable-system-include-path -Wthread-safety -MD -MT libc/test/src/__support/FPUtil/CMakeFiles/libc.test.src.__support.FPUtil.bfloat16_test.__unit__.__build__.dir/bfloat16_test.cpp.o -MF libc/test/src/__support/FPUtil/CMakeFiles/libc.test.src.__support.FPUtil.bfloat16_test.__unit__.__build__.dir/bfloat16_test.cpp.o.d -o libc/test/src/__support/FPUtil/CMakeFiles/libc.test.src.__support.FPUtil.bfloat16_test.__unit__.__build__.dir/bfloat16_test.cpp.o -c /home/mgadelha/tools/llvm-project/libc/test/src/__support/FPUtil/bfloat16_test.cpp
In file included from /home/mgadelha/tools/llvm-project/libc/test/src/__support/FPUtil/bfloat16_test.cpp:12:
In file included from /home/mgadelha/tools/llvm-project/libc/utils/MPFRWrapper/MPCommon.h:21:
In file included from /home/mgadelha/tools/llvm-project/libc/utils/MPFRWrapper/mpfr_inc.h:20:
In file included from /usr/include/mpfr.h:53:
In file included from /usr/include/x86_64-linux-gnu/gmp.h:35:
In file included from /usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/iosfwd:38:
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/requires_hosted.h:34:4: error: "This header is not available in freestanding mode."
34 | # error "This header is not available in freestanding mode."
| ^
In file included from /home/mgadelha/tools/llvm-project/libc/test/src/__support/FPUtil/bfloat16_test.cpp:12:
In file included from /home/mgadelha/tools/llvm-project/libc/utils/MPFRWrapper/MPCommon.h:21:
In file included from /home/mgadelha/tools/llvm-project/libc/utils/MPFRWrapper/mpfr_inc.h:20:
In file included from /usr/include/mpfr.h:53:
In file included from /usr/include/x86_64-linux-gnu/gmp.h:36:
In file included from /usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:42:
In file included from /home/mgadelha/tools/llvm-project/buildlibc/libc/include/stdio.h:14:
In file included from /home/mgadelha/tools/llvm-project/buildlibc/libc/include/llvm-libc-macros/stdio-macros.h:12:
/home/mgadelha/tools/llvm-project/buildlibc/libc/include/llvm-libc-macros/../llvm-libc-types/FILE.h:12:16: error: typedef 'FILE' cannot be referenced with the 'struct' specifier
12 | typedef struct FILE FILE;
| ^
/usr/include/x86_64-linux-gnu/bits/types/FILE.h:7:25: note: declared here
7 | typedef struct _IO_FILE FILE;
| ^
In file included from /home/mgadelha/tools/llvm-project/libc/test/src/__support/FPUtil/bfloat16_test.cpp:12:
In file included from /home/mgadelha/tools/llvm-project/libc/utils/MPFRWrapper/MPCommon.h:21:
In file included from /home/mgadelha/tools/llvm-project/libc/utils/MPFRWrapper/mpfr_inc.h:20:
In file included from /usr/include/mpfr.h:53:
In file included from /usr/include/x86_64-linux-gnu/gmp.h:36:
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:99:11: error: no member named 'fpos_t' in the global namespace
99 | using ::fpos_t;
| ~~^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:107:11: error: no member named 'fgetpos' in the global namespace
107 | using ::fgetpos;
| ~~^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:114:11: error: no member named 'freopen' in the global namespace
114 | using ::freopen;
| ~~^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:117:11: error: no member named 'fsetpos' in the global namespace
117 | using ::fsetpos;
| ~~^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:133:11: error: no member named 'rewind' in the global namespace
133 | using ::rewind;
| ~~^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:139:11: error: no member named 'tmpfile' in the global namespace
139 | using ::tmpfile;
| ~~^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:141:11: error: no member named 'tmpnam' in the global namespace
141 | using ::tmpnam;
| ~~^
In file included from /home/mgadelha/tools/llvm-project/libc/test/src/__support/FPUtil/bfloat16_test.cpp:12:
/home/mgadelha/tools/llvm-project/libc/utils/MPFRWrapper/MPCommon.h:174:5: error: use of undeclared identifier 'mpfr_set_sj'
174 | mpfr_set_sj(value, x, mpfr_rounding);
| ^~~~~~~~~~~
/home/mgadelha/tools/llvm-project/libc/test/src/__support/FPUtil/bfloat16_test.cpp:65:28: note: in instantiation of function template specialization '__llvm_libc_22_0_0_git::testing::mpfr::MPFRNumber::MPFRNumber<int, 0>' requested here
65 | BFloat16 mpfr_bfloat = MPFRNumber(i).as<BFloat16>();
| ^
10 errors generated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another alternative would be to change bfloat16_test.cpp
so it doesn't use mpfr, but I think this should be a bit more complicated.
How do you want me to proceed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you temporarily disable bfloat16_test
for full build mode? It is the MPCommon
target that needs to be updated so that mpfr_inc.h
is not included in the MPCommon.h
header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll submit a different PR for that!
If we are doing a full libc build, mpfr tests should be disabled even if we set the mpfr path. This patch simply swaps the check order.
This should fix the rv32 buildbot.